Miscellaneous Pipeline Changes #326

PlagueHO · 2020-04-27T04:02:19Z

Pull Request (PR) description

Update ModuleBuilder to Latest.
Merge HISTORIC_CHANGELOG.md into CHANGELOG.md.
Change Azure DevOps Pipeline trigger to include source/*.

This PR also includes a fix for #323.

I also a known issue info to address #294.

This Pull Request (PR) fixes the following issues

Task list

Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
Entry should say what was changed, and how that affects users (if applicable).
Resource documentation added/updated in README.md in resource folder.
Resource parameter descriptions added/updated in schema.mof
and comment-based help.
Comment-based help added/updated.
Localization strings added/updated in all localization files as appropriate.
Examples appropriately added/updated.
Unit tests added/updated. See DSC Resource Testing Guidelines.
Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

@johlju - would you mind reviewing for me?

This change is

PlagueHO · 2020-04-27T05:26:22Z

Hold off on this one @johlju - I'm not sure why Integration tests are failing. There wasn't any change that would have affected that so it may be something that has changed with the agent.

johlju · 2020-04-27T07:11:43Z

No worries. Let me know when it is ready to review. 🙂

…terManagementDsc into Issue-324-and-325

PlagueHO · 2020-05-04T09:21:49Z

Hi @johlju - this PR is now ready to review. I had to make some additional changes to get the tests to pass.

johlju

Reviewed 3 of 4 files at r1, 6 of 6 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @PlagueHO)

source/DSCResources/DSC_ScheduledTask/DSC_ScheduledTask.psm1, line 1955 at r3 (raw file):

-not $force -and

Shouldn't this be $force -and (remove -not) otherwise this will be done when Force -eq $false.

Looks like there is no unit test for that scenario so it wasn't caught. 🙂

PlagueHO

Reviewable status: 7 of 9 files reviewed, 1 unresolved discussion (waiting on @johlju)

source/DSCResources/DSC_ScheduledTask/DSC_ScheduledTask.psm1, line 1955 at r3 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

-not $force -and
Shouldn't this be $force -and (remove -not) otherwise this will be done when Force -eq $false.

Looks like there is no unit test for that scenario so it wasn't caught. 🙂

The logic is actually correct based on the intent, but due to the double negatives isn't obvious. So I added a comment to clarify. I also added one more test to confirm.

johlju

All good. Didn't look at the logic close enough. 🙂

Reviewed 2 of 2 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved

johlju · 2020-05-05T05:00:27Z

Closing and reopening to kick of the tests again since it failed on. I have seen this when the previous run is to fast to the LCM haven't time to cooldown.

CimException: Cannot invoke the Get-DscConfiguration cmdlet. The Consistency Check or Pull
cmdlet is in progress and must return before Get-DscConfiguration can be invoked. Use -Force 
option if that is available to cancel the current operation.

https://dev.azure.com/dsccommunity/ComputerManagementDsc/_build/results?buildId=1818&view=logs&j=bed57fa5-efa2-5e6c-3967-59915169b168&t=5a0501c2-c194-508e-8f56-358cb6bbc353&l=383

In xWebAdministration and SqlServerDsc I have added the following helper function between tests where this happens (not everywhere though). Might be better to add it to Initialize-TestEnvironment.

https://github.com/dsccommunity/SqlServerDsc/blob/fab9419549a490efab38e070e8eda40d2a10d531/tests/TestHelpers/CommonTestHelper.psm1#L398-L409

PlagueHO · 2020-05-05T05:57:38Z

Thanks for that @johlju . In other resources we used to use the Reset-Dsc function that was part of the Test initialization and/or test helper, but this got eliminated by the move to the new CI process. I meant to add something back in but haven't had time. I created this many years back for xPSDesiredStateConfiguration I think. Anyway, I'll aim to roll your fix in.

PlagueHO · 2020-05-05T06:03:57Z

Sidenote: the reason why we decided to force the LCM to stop (Reset-Dsc) rather than to wait is that the LCM on occasion took a long time to complete so forcing a stop resulted in faster test completion in these situations. xPSDesiredStateConfiguration was very problematic here (often jamming up multiple times in a single CI run).

johlju · 2020-05-05T06:10:17Z

Aha, so maybe that is why we are starting to see these error because "Reset-Dsc" was removed. I'm good with Reset-Dsc too if it helps tests to run faster. But the Wait-ForIdleLcm has worked perfectly. I seen it at the most wait up to 8 seconds to continue (4 retries). Though, that is 8 seconds we could save with Reset-Dsc. 🙂

PlagueHO · 2020-05-05T07:04:16Z

I'm OK either way - but it would be good if we could unify the functions everywhere. E.g. Perhaps create a DscResource.TestHelper module that contains some of this shared code. Not that it is really a big deal replicating it between modules.

Anyway, I've raised some issues in they key repos where this is a problem and will address when I get time (need to do some more module conversions to new CI tonight).

johlju · 2020-05-05T08:18:58Z

I agree a test helper module would be great, I thought about that too.

Fix Issue dsccommunity#324 and dsccommunity#325

83d3b36

PlagueHO added the needs review The pull request needs a code review. label Apr 27, 2020

Merge branch 'master' into Issue-324-and-325

a0c83e8

johlju added waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. and removed needs review The pull request needs a code review. labels Apr 27, 2020

PlagueHO mentioned this pull request May 3, 2020

ScheduledTask: Resource is never in compliance when BuiltInAccount is used #323

Closed

PlagueHO added 22 commits May 3, 2020 17:43

Fix Issue dsccommunity#323

6b02ed9

Merge branch 'Issue-324-and-325' of https://github.com/PlagueHO/Compu…

a4bedc8

…terManagementDsc into Issue-324-and-325

Correct builtInAccount behavior

1ec54a4

Fix BuiltInAccount handling

c05a32d

Add Debug Info

0f4dc4c

Another try

75b7413

Remove debugging code

ead9453

Add some debug

3a4df5e

Fix integration tests

2c2f47a

Fix ScheduledTask integration test config names

a349ec0

Improve integration tests

1119076

Improve integration tests of ScheduledTask

32efe93

Test changing Users group name

c46ccc4

Implement Set-DomainNameInAccountName function

d9024fd

Fix typo in ScheduledTask BuiltInAccount handling

3e7ac11

Improve ScheduledTask Code structure

bda5751

Add fixe for dsccommunity#294

634fbce

Remove debug code

def95e3

Fix PendingReboot tests

b618df7

Fix PendingReboot parameters

a4c3df9

Fix PendingReboot integration tests

88e0388

Another fix to PendingReboot Integration tests

d09392c

Update CHANGELOG

8db44ab

johlju added needs review The pull request needs a code review. and removed waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. labels May 4, 2020

johlju requested changes May 4, 2020

View reviewed changes

johlju added waiting for code fix A review left open comments, and the pull request is waiting for changes to be pushed by the author. and removed needs review The pull request needs a code review. labels May 4, 2020

Clarify logic

74eeeb8

PlagueHO commented May 5, 2020

View reviewed changes

johlju approved these changes May 5, 2020

View reviewed changes

johlju closed this May 5, 2020

johlju reopened this May 5, 2020

PlagueHO merged commit 29db49a into dsccommunity:master May 5, 2020

PlagueHO deleted the Issue-324-and-325 branch May 5, 2020 06:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Miscellaneous Pipeline Changes #326

Miscellaneous Pipeline Changes #326

PlagueHO commented Apr 27, 2020 •

edited

Loading

PlagueHO commented Apr 27, 2020

johlju commented Apr 27, 2020

PlagueHO commented May 4, 2020

johlju left a comment

PlagueHO left a comment

johlju left a comment

johlju commented May 5, 2020

PlagueHO commented May 5, 2020

PlagueHO commented May 5, 2020

johlju commented May 5, 2020 •

edited

Loading

PlagueHO commented May 5, 2020

johlju commented May 5, 2020

Miscellaneous Pipeline Changes #326

Miscellaneous Pipeline Changes #326

Conversation

PlagueHO commented Apr 27, 2020 • edited Loading

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

Task list

PlagueHO commented Apr 27, 2020

johlju commented Apr 27, 2020

PlagueHO commented May 4, 2020

johlju left a comment

Choose a reason for hiding this comment

PlagueHO left a comment

Choose a reason for hiding this comment

johlju left a comment

Choose a reason for hiding this comment

johlju commented May 5, 2020

PlagueHO commented May 5, 2020

PlagueHO commented May 5, 2020

johlju commented May 5, 2020 • edited Loading

PlagueHO commented May 5, 2020

johlju commented May 5, 2020

PlagueHO commented Apr 27, 2020 •

edited

Loading

johlju commented May 5, 2020 •

edited

Loading